servers/build/build_saga_test.go (2)

109-124: ⚡ Quick win

Make stub artifact names unique per run.

Using a fixed <app>-stub entity name makes this helper fragile if a test starts the saga more than once against the same in-memory store.

♻️ Proposed fix

-	artifactName := in.AppName + "-stub"
+	artifactName := in.AppName + "-" + in.VersionName + "-stub"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@servers/build/build_saga_test.go` around lines 109 - 124, The helper
currently uses a fixed artifactName ("artifactName := in.AppName + \"-stub\"")
which causes collisions when the saga is started multiple times against the same
in-memory store; change artifactName generation in the build helper to include a
run-unique suffix (e.g., append time.Now().UnixNano() or a generated UUID)
before calling deps.builder.ec.Create so each created entity name is unique and
avoids test interference with artifact creation via deps.builder.ec.Create.

---

293-309: ⚡ Quick win

Assert compensation by checking entity absence, not only UndoneAt.

This currently verifies saga metadata only. Add direct store checks for the created ConfigVersion/AppVersion entities to ensure compensators actually removed persisted rows.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@servers/build/build_saga_test.go` around lines 293 - 309, The test currently
only checks
exec.ExecutedActions[actionCreateConfigVer|actionCreateVersion].UndoneAt; extend
it to assert the compensators actually removed persisted rows by attempting to
load the corresponding entities (ConfigVersion and AppVersion) from the store
and expecting a "not found" result. After you retrieve exec from storage.Get,
derive the created entity IDs from the execution record or action payload, then
call the store read methods for ConfigVersion and AppVersion (use the project's
existing load/get functions) and assert they return a not-found error (or nil
result) rather than a valid entity; keep these checks adjacent to the existing
UndoneAt assertions for actionCreateConfigVer and actionCreateVersion.

servers/build/build_saga.go (1)

659-689: 💤 Low value

Minor: Unused variable workaround can be removed when migration check is updated.

Line 675 has _ = status to suppress the unused variable warning. The comment at lines 667-672 explains this is a temporary regression. Consider tracking this as technical debt.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@servers/build/build_saga.go` around lines 659 - 689, The _ = status no-op
exists to silence an unused variable in finalize; remove this workaround (delete
the `_ = status` line) and either use status where intended or add a short TODO
comment referencing the pending migration work so the linter won't be suppressed
silently; locate this in the finalize function (symbols: finalize, status,
checkLocalStorageMigration) and ensure no other unused-variable warnings remain
after removal.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

servers/build/build_saga.go (1)

438-442: 💤 Low value

Consider logging a warning if EphemeralExpiresAt fails to parse.

The parse error is silently ignored. While this value comes from handleEphemeral which formats with RFC3339 (so it should never fail), silent failures could mask data corruption issues where ephemeral versions never expire.

Suggested enhancement

 		if in.EphemeralExpiresAt != "" {
-			if t, err := time.Parse(time.RFC3339, in.EphemeralExpiresAt); err == nil {
+			t, err := time.Parse(time.RFC3339, in.EphemeralExpiresAt)
+			if err != nil {
+				b.Log.Warn("failed to parse ephemeral expiration time", "value", in.EphemeralExpiresAt, "error", err)
+			} else {
 				av.EphemeralExpiresAt = t
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@servers/build/build_saga.go` around lines 438 - 442, The parse error for
in.EphemeralExpiresAt is currently ignored; update the time.Parse branch to log
a warning when parsing fails (include the offending in.EphemeralExpiresAt string
and the parse error) while preserving current behavior when parse succeeds (set
av.EphemeralExpiresAt = t). Use the package's logger (or the standard log
package) to emit the warning from the same context where in.EphemeralExpiresAt
and av.EphemeralExpiresAt are handled.